👍

Does this site selection process differ from what David already implemented in the downscaling repo? Seems preferable to use an existing tool rather than add that complexity to the UA process.

I have refactored this script to completely remove the clustering logic. Instead it now acts purely as a consumer of the shared design_points.csv artifact (currently the david's manually selected 198 sites) to perform the necessary PFT mapping and sub-sampling for UA.

We will treat this as the interface for now, and can revisit or refactor the integration logic once the broader coordination strategy between the Downscaling and Uncertainty workflows is finalized.

see comment #1 (comment)

I'm not sure what "local" means here

good catch regarding the ambiguity

In this context, 'Local' refers to the mathematical method used by pecan's variance decomposition one-at-a-time (OAT) SA, where parameters are varied individually around their median to calculate partial derivatives (elasticities). This is distinct from the global sensitivity analysis we perform later.

I have updated the function documentation to explicitly state this aggregates one-at-a-time (OAT) parameter sensitivity results to avoid confusion with geographical locality.

I recommend documenting what columns are expected in this df, and whether unexpected columns are ignored or cause errors

👍 documented

This will be much easier to read and understand if you define it as a function with an informative name -- yes, even if it's only called once as all_results <- purrr::map_dfr(sa_files, name_of_function)

I have extracted the file processing logic into a standalone helper function process_sa_file

.

The goal with this file was to handle settings that aren't in the pecan xml. Is there a reason that these are included here rather than the template.xml?

Minimizing config options here, and providing sensible defaults in the template.xml could make it more clear to end users.

Once we have ccmmf_dir, can we change this to use that as a variable? That way it is only necessary to change the system-specific path once. (also applies to master_design points etc. )

nit: these are just design points ...

These may seem like trivial naming requests, but clarity and specificity is important.

• why is this 'master_design_points' ... that implies that there are other sets of design points.
• I would call this object design_points_csv instead of master_file
• instead of master_data, I would call that object design_points.

why are we sampling here?

these were not manually selected, they were from the site selection

The fact that this script exists makes me wonder whether the clustering workflow that generates design_points.csv should be updated - that will help ensure consistency in workflows that consume it.

it is unclear - why are configs hard coded here if they are already in the config file and/or template.xml?

consider - what is the minimum amount of information that we want the end users to be able to configure, and why?

In addition to file paths, I can see n_sample, start_date, end_date being useful for being able to trigger 'development' mode.

is it worth the effort and overhead to manage a STATUS file? My recollection is that the STATUS file was developed primarily for the PHP web interface to display. It seems that dropping the status file would simplify the code a lot. What benefit does it provide, if any?

This is a context where targets might be useful.

It would be helpful to be able to view all of these plots at the same time, either as panels or different color bars.

By default, I prefer to show medians, especially for non-normally distributed values. If they exist, consider showing the distribution of values (e.g., across sites)

The site-by-site table is difficult to interpret and doesn't add much once across site variance is shown in the elasticity plot. Lets remove it. Andrew Gelman’s “Let’s practice what we preach: turning tables into graphs” makes a good case for visual summaries when tables get this dense: https://sites.stat.columbia.edu/gelman/research/published/dodhia.pdf

(The reason that I put a dense table of county aggregated means in the downscaling report is that those values are the numbers that the client wants)

A map would be a nice way to show the distribution of sites. It could eventually (not required for MVP) be interesting to map elasticity of the first, say, five parameters.

A few small usability notes for the style of table:

• when rendered as HTML, the table scrolls horizontally but headers don’t stay aligned.
• Two significant figures likely be enough; additional precision is noise.
• The cell color bars are helpful for relative magnitude.

Consider something like a heat map. it isn't necessary to show every site, but perhaps summarize by environmental cluster?

See Dietze et al 2017 https://agupubs.onlinelibrary.wiley.com/doi/10.1002/2013JG002392

link to this section above, or perhaps it could be in the beginning.

its not clear why there are only two unique values of r2 in the table

this plot has two rows with the same values

one way these could be displayed is using facets

• each row a different parameter (for top params / greatest slopes)
• each column a different environmental covariate
• each plot has x = env covariate and y = elasticity. Could use color to plot multiple outputs (SOC, NPP, etc) on the same plot, or create separate plots for each output.

Another note on plots - by default, keep the axes (and other scales) the same size even across different variations of a plot (e.g. when all elasticity should have the same range. Sometimes it is necessary to change this role - e.g. to show interesting trends that would otherwise be obscured. This makes it easier to compare.

taking the absolute value of elasticity makes sense when ranking parameter importance, but for plotting and regression, we should use untransformed elasticity.

There are lots of ways to plot this information but this is from LeBauer et al 2013 (shown here mostly to note that the direction of elasticity is also important information).